feat: add commitBehavior to NumberField#9679
Conversation
|
Happy to have a stab at the docs but I'd like to please know if this is something you'd consider adding first 🙏 😅 |
|
@will-stone interesting, this went a bit of a different route than I was expecting but I suppose native number type input fields do behave the same way. @snowystinger Any opinions here? Do you remember any reasons we didn't go this route in the first place? |
|
Hi 👋 🙂 Anything I can do to advance this? If you're happy with the name then I can have a crack at the docs. |
|
@will-stone sorry about the delay, I'll take a look soon. I hadn't realized the team had already discussed this a while back haha, I'll take a look at overall behavior soon. Definitely feel free to take a crack at the docs, the team can help out with those if behavior needs to be refactored/naming gets changed/etc |
| /** | ||
| * Disables value snapping when user finishes editing the value (e.g. on blur). | ||
| */ | ||
| isValueSnappingDisabled?: boolean |
There was a problem hiding this comment.
such a long prop name, how would we feel instead about, question to the team
isSnapDisabled
There was a problem hiding this comment.
I think that shorter name is fine
There was a problem hiding this comment.
perhaps interactOutside behavior related? like RangeCalendar? and DatePicker?
See #8899 (review) as well
| expect(onChangeSpy).toHaveBeenCalledWith(5); | ||
| expect(textField).toHaveAttribute('value', '5'); | ||
|
|
||
| expect(container).not.toHaveAttribute('aria-invalid'); |
There was a problem hiding this comment.
maybe in the v3 spectrum component this should be false, but if we write the same test in the react aria component, does it have the invalid attribute? seems like it should
| /** | ||
| * Disables value snapping when user finishes editing the value (e.g. on blur). | ||
| */ | ||
| isValueSnappingDisabled?: boolean |
There was a problem hiding this comment.
I think that shorter name is fine
snowystinger
changed the title
snowystinger
changed the title
| export const InteractOutsideBehaviorNone: NumberFieldStory = () => render({step: 3, minValue: 2, maxValue: 21, commitBehavior: 'validate'}); | ||
|
|
||
| InteractOutsideBehaviorNone.story = { | ||
| name: 'commitBehavior = validate' | ||
| }; |
There was a problem hiding this comment.
| export const InteractOutsideBehaviorNone: NumberFieldStory = () => render({step: 3, minValue: 2, maxValue: 21, commitBehavior: 'validate'}); | |
| InteractOutsideBehaviorNone.story = { | |
| name: 'commitBehavior = validate' | |
| }; | |
| export const CommitBehaviorValidate: NumberFieldStory = () => render({step: 3, minValue: 2, maxValue: 21, commitBehavior: 'validate'}); | |
| CommitBehaviorValidate.story = { | |
| name: 'commitBehavior = validate' | |
| }; |
LFDanLu
left a comment
LFDanLu
left a comment
There was a problem hiding this comment.
Verified the validation behavior except for the native localizations since Chrome on Mac can't override the display language, will test that on Windows later
…-snapping-disabled-to-number-field
|
Thank you everyone! 😄 |
6 participants
As discussed here: #8776
✅ Pull Request Checklist:
📝 Test Instructions:
Step3WithMin2Max21ValueSnappingDisabledstory.onChangecallback fires with the same value you entered, and the input does not auto-snap to a valid number.🧢 Your Project:
This will be used in an e-commerce situation where the
NumberFieldis used for product quantity. Snapping is dangerous, for us, because a customer may not know that the number has been auto-healed, causing them to order less or more of what they need. I work for RS Group, and RAC powers our component library: ion. The component in question isn't live yet, but the RAC implementation is to replace ourQuantitySteppercomponent.